-
Notifications
You must be signed in to change notification settings - Fork 24.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added support for textDecorationLine style prop on Android #5105
Conversation
mIsUnderlineTextDecorationSet = ( | ||
"underline".equals(textDecorationLineString) || | ||
"underline line-through".equals(textDecorationLineString)); | ||
mIsLineThroughTextDecorationSet = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a conscious decision or some sort of standard to only support "underline line-though" but not "line-through underline"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was just to be consistent with the iOS version.
@Charca updated the pull request. |
@kmagiera latest update lets us support bot |
Oh cool! Should we close #3819? @Charca can you rebase please? @andreicoman11 Can you review? |
04598a4
to
34d021c
Compare
@Charca updated the pull request. |
👍 Thanks @Charca. |
@mkonicek would you mind taking a look at this pull request? It's been a while since the last commit was reviewed. |
What's about dashed/dotted underline? |
I'll let @kmagiera or @andreicoman11 review this one. |
@@ -428,6 +439,18 @@ public void setFontStyle(@Nullable String fontStyleString) { | |||
} | |||
} | |||
|
|||
@ReactProp(name = ViewProps.TEXT_DECORATION_LINE) | |||
public void setTextDecorationLine(@Nullable String textDecorationLineString) { | |||
for(String textDecorationLineSubString:textDecorationLineString.split(" ")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if textDecorationLineString
is null?
34d021c
to
011c026
Compare
@Charca updated the pull request. |
@mkonicek would you mind taking a look at this pull request? It's been a while since the last commit was reviewed. |
|
||
UnderlineSpan underlineSpan = | ||
getSingleSpan((TextView) rootView.getChildAt(0), UnderlineSpan.class); | ||
assertThat(underlineSpan instanceof UnderlineSpan).isTrue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert other Span is false
011c026
to
e84e2ac
Compare
@Charca updated the pull request. |
Looks good! Thank you! |
@facebook-github-bot shipit |
Thanks for importing. If you are an FB employee go to Phabricator to review. |
2039be9
Summary:As suggested by kmagiera in facebook#3819, I've implemented `textDecorationLine` style for Android in `ReactTextShadowNode` using span operations so we can support nested Text components. ![Demo](http://c.hlp.sc/022k2l033p0n/Image%202016-01-03%20at%2011.17.15%20PM.png) Closes facebook#5105 Differential Revision: D3167756 Pulled By: andreicoman11 fb-gh-sync-id: 122701a53d50f47f89b49e1f343c97db5fa2323d fbshipit-source-id: 122701a53d50f47f89b49e1f343c97db5fa2323d
As suggested by @kmagiera in #3819, I've implemented
textDecorationLine
style for Android inReactTextShadowNode
using span operations so we can support nested Text components.Demo